Skip to content

[ refactor ] downstream improvements following #2961 / #2968#2970

Draft
jamesmckinna wants to merge 13 commits intoagda:masterfrom
jamesmckinna:refactor-AVL-downstream
Draft

[ refactor ] downstream improvements following #2961 / #2968#2970
jamesmckinna wants to merge 13 commits intoagda:masterfrom
jamesmckinna:refactor-AVL-downstream

Conversation

@jamesmckinna
Copy link
Copy Markdown
Collaborator

@jamesmckinna jamesmckinna commented Mar 25, 2026

DRAFT: for rebasing against #2968 in due course. If #2968 does indeed get merged, then this PR revolves entirely around the properties of Map.Membership, and we may wish to revisit/refactorthat separately (under the usual Setoid/Propositional distinction?)

Notes to self (and others):

@jamesmckinna jamesmckinna marked this pull request as draft March 25, 2026 14:57
@jamesmckinna jamesmckinna added status: blocked-by-issue Progress on this issue or PR is blocked by another issue. refactoring labels Mar 25, 2026
@jamesmckinna jamesmckinna added this to the v2.4 milestone Mar 25, 2026
Please use insertWith-nothing instead."
#-}
Any-insertWith-just = insertWith-just
{-# WARNING_ON_USAGE Any-insertWith-just
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like Any-insertWith-just isn't general enough, which might be related to deprecating with name changes. Not sure if there's a standard practice for how to chose new names when the old lemma isn't general enough, but I thought I'd mention it here.

The problem with Any-insertWith-just is (pr : ∀ k′ v → (eq : k ≈ k′) → P (k′ , Val≈ eq (f (just (Val≈ (sym eq) v))))). See that v is not constrained to be the preexisting v in the old tree. So this is good enough to prove Any-insert-just, which ignores the v anyway, but doesn't seem useful when insertWith is used with a non-trivial function.

One possible fix is:

Any-insertWith-just-strong :
  (∀ k′ v → (eq : k ≈ k′) → Q (k′ , v) → P (k′ , Val≈ eq (f (just (Val≈ (sym eq) v))))) →
  (q : Any Q t) → k ≈ lookupKey q →
  Any P (proj₂ (insertWith k f t seg))

The proof of this happens to be almost exactly a copy and paste of the existing Any-insertWith-just.

Copy link
Copy Markdown
Collaborator Author

@jamesmckinna jamesmckinna Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks very much.
The thing to do in such circumstances would be to fix the type of the 'new' functions, and deprecate the old, but noting that the type has improved. At least if I recall previous episodes of this character... will look properly in the morning soon!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... I've been thinking about your comment a bit more (but not really doing any proof), and your generalisation, while it seems stronger, perhaps may also be refactored via the existing proof and Properties.Lookup.lookup-rebuild?

@jamesmckinna jamesmckinna removed this from the v2.4 milestone Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring status: blocked-by-issue Progress on this issue or PR is blocked by another issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants